Skip to content

Comments

서현하 sprint4#76

Merged
bellman66 merged 12 commits intocodeit-bootcamp-spring:서현하from
by15622:서현하_sprint4
Feb 24, 2026

Hidden character warning

The head ref may contain hidden characters: "\uc11c\ud604\ud558_sprint4"
Merged

서현하 sprint4#76
bellman66 merged 12 commits intocodeit-bootcamp-spring:서현하from
by15622:서현하_sprint4

Conversation

@by15622
Copy link
Collaborator

@by15622 by15622 commented Feb 14, 2026

요구사항

기본

  • 기본 항목 1
  • 기본 항목 2

심화

  • 심화 항목 1
  • 심화 항목 2

주요 변경사항

스크린샷

image

멘토에게

  • 심화는 완성하지 못했습니다.
  • BasicBinaryContentService 바이트 형식으로 포스트맨에서 텍스트는 출력이 되었는데 이미지는 출력이 안되어서 넣지 못했습니다

Comment on lines +16 to +19
public ResponseEntity<String> handleException(IllegalArgumentException e) {
return ResponseEntity
.status(HttpStatus.BAD_REQUEST)
.body(e.getMessage());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반환전에 Log를 남기는 로직을 체크해주세요.
서버의 입장에서는 반환 후 어떤 에러가 도출되는지 알지 못합니다.
log 구현체를 사용해서 시간 + 에러 내용을 기록하는 방법을 지원해주세요.

Comment on lines +22 to +27
@ExceptionHandler(NoSuchElementException.class)
public ResponseEntity<String> handleException(NoSuchElementException e) {
return ResponseEntity
.status(HttpStatus.NOT_FOUND)
.body(e.getMessage());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 Exception Handler에서는 Exception의 메세지가 그대로 반환하고 있습니다.
이 부분을 그대로 반환하게 된다면 유저 입장에서 시스템 에러를 읽을 수 있습니다.
어떤 에러인지 분기해서 내려주는게 어떨까요

Ex)
.body("값이 존재하지 않습니다.")

ConfigurableApplicationContext context = SpringApplication.run(DiscodeitApplication.class, args);

UserService userService = context.getBean(UserService.class);
/* UserService userService = context.getBean(UserService.class);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석보다 테스트 로직을 삭제하는 편이 좋아보입니다.

주석의 경우 차후 미사용하거나 잊혀지는 경우가 많아서 제거하거나
따로 Test 파일을 만들어서 관리하는 편이 좋습니다.

Comment on lines +31 to +32
status.update("ONLINE");
userStatusRepository.save(status);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로직상 궁금한 점이 있어 코멘트 드립니다.

로그인시 ONLINE으로 전환하도록 세팅이 되어있는데요.
만약에 정상적으로 로그아웃하는 상황이라면 OFFLINE으로 세팅을 하겠지만
그냥 유저가 꺼버리는 경우는 OFFLINE으로 어떻게 세팅이 해줄수 있을까요

Comment on lines +66 to 67
} else {
try (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else 를 없애도 상관없을거 같습니다

Comment on lines +26 to +28
} catch (IOException e) {
throw new RuntimeException(e);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RuntimeException으로 묶여있는 상태인데요.
Exception handler에서 Excepion으로 처리되면서 다른 일반 Exception과 혼용될 가능성이 높습니다.

이런 경우 따로 Exception으로 만들거나 IO를 따로 핸들링하는 방식으로 전환해주세요

Comment on lines +16 to +18
public List<ReadStatus> findAllByChannelId(UUID channelId) {
return List.of();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

따로 구현이 안되어있는 상태로 보입니다. 차후에 추가해주세요.

Comment on lines +19 to +20
private boolean online; //
private Instant lastActiveAt; //
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

필드 뒤에 주석 줄은 제거해주세요 ( // )

request.contentType(),
(long) originalBytes.length,
originalBytes
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BinaryContent 만드는 경우에 실질적으로 request 내부 값들을 통해서 만들어지게 되는데요.
이런 경우 request 객체로 매핑된 값을 따로 날려주면 좀더 편하게 생성 로직을 제시할수 있습니다.

참고로 BinaryContentCreateRequest를 직접적으로 쓰는 것보다는 따로 DTO를 만들어서 쓰는 경우가 좋습니다.
-> BinaryContentCreator DTO 생성 후 매핑

Ex) new BinaryContent(request) or new BinaryContent(creator)


@Override
public List<Channel> findAllByUserId(UUID userId) {
public List<ChannelDto> findAllByUserId(UUID userId) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTO 사용 좋습니다 👍 👍

Comment on lines +54 to +59
.filter(channel -> {
if (channel.getType() == ChannelType.PUBLIC) return true;

return readStatusRepository.findAllByChannelId(channel.getId()).stream()
.anyMatch(rs -> rs.getUserId().equals(userId));
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter내 repository 조회하는 부분을 제거해주세요.

channel 개수만큼 조회를 하게되면서 성능상 문제를 가져올 가능성이 높습니다.

.orElseThrow(() -> new RuntimeException("해당 유저의 상태 정보를 찾을 수 없습니다."));
.orElseGet(() -> {
return new UserStatus(userId);
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orElseGet 으로 Default 값을 만들어서 보내주게 되는데요
값이 실제로 있는지 착각할 가능성도 있어 보입니다.

예를 들어 조회를 한 경우에 실질적으로 값이 없지만 디폴트 값을 주면서 값이 도출하게 됩니다.
이런 경우 Entity내 상태값이 표현하거나 Throw를 던지는 편이 좋아보입니다.

@PostMapping
public ResponseEntity<BinaryContent> create(@RequestBody BinaryContentCreateRequest request) {
BinaryContent savedContent = binaryContentService.create(request);
return ResponseEntity.status(HttpStatus.CREATED).body(savedContent);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BinaryContent Entity를 그대로 반환하지 말고 다른 DTO등으로 감싸주세요.

해당 Entity 내부에 byte코드도 같이 json으로 반환하게 되면서 부하가 생깁니다.

Comment on lines 86 to 94
@@ -86,8 +90,5 @@ public void delete(UUID userId) {
userStatusRepository.deleteByUserId(userId);
userRepository.deleteById(userId);
}

}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생성시 profile을 바당서 생성하도록 하는데요. 삭제로직에서 해당 프로파일을
지우는 로직이 없는거 같습니다. 로직을 검토해주세요

Copy link
Collaborator

@bellman66 bellman66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트 추가하였습니다. 코멘트대로 업데이트 요청드립니다.

@bellman66 bellman66 merged commit 8a7e4d9 into codeit-bootcamp-spring:서현하 Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants